repo: Make ostree_repo_create() nonfatal on existing repos

In general we want to support "idempotentcy" or "state
synchronization" across interruption.  If a repo is only partially
created due to a crash or whatever, it's hard for a user to know that.
Let's just make `ostree_repo_create()` idempotent. Since all we're
doing is a set of `mkdirat()` invocations, it's quite simple.

This also involved porting to fd-relative, which IMO makes the
code a lot clearer.

Closes: #422
Approved by: 14rcole

diff --git a/src/libostree/ostree-repo.c b/src/libostree/ostree-repo.c
index 89d976b2a391ff8bdf6885b3c4a5a8e3fc097e4c..c48c139e1590a57f3707eee36f71dc957c688b51 100644 (file)
--- a/src/libostree/ostree-repo.c
+++ b/src/libostree/ostree-repo.c
@@ -1842,8 +1842,15 @@ ostree_repo_mode_from_string (const char      *mode,
  * @cancellable: Cancellable
  * @error: Error
  *
- * Create the underlying structure on disk for the
- * repository.
+ * Create the underlying structure on disk for the repository, and call
+ * ostree_repo_open() on the result, preparing it for use.
+
+ * Since version 2016.8, this function will succeed on an existing
+ * repository, and finish creating any necessary files in a partially
+ * created repository.  However, this function cannot change the mode
+ * of an existing repository, and will silently ignore an attempt to
+ * do so.
+ *
  */
 gboolean
 ostree_repo_create (OstreeRepo     *self,
@@ -1851,76 +1858,65 @@ ostree_repo_create (OstreeRepo     *self,
                     GCancellable   *cancellable,
                     GError        **error)
 {
-  gboolean ret = FALSE;
-  GString *config_data = NULL;
-  g_autoptr(GFile) child = NULL;
-  g_autoptr(GFile) grandchild = NULL;
-  const char *mode_str;
-
-  if (!ostree_repo_mode_to_string (mode, &mode_str, error))
-    goto out;
+  const char *repopath = gs_file_get_path_cached (self->repodir);
+  glnx_fd_close int dfd = -1;
+  struct stat stbuf;
+  const char *state_dirs[] = { "objects", "tmp", "extensions", "state",
+                               "refs", "refs/heads", "refs/remotes" };
 
-  if (mkdir (gs_file_get_path_cached (self->repodir), 0755) != 0)
+  if (mkdir (repopath, 0755) != 0)
     {
-      if (errno != EEXIST)
+      if (G_UNLIKELY (errno != EEXIST))
         {
           glnx_set_error_from_errno (error);
-          goto out;
+          return FALSE;
         }
     }
 
-  config_data = g_string_new (DEFAULT_CONFIG_CONTENTS);
-  g_string_append_printf (config_data, "mode=%s\n", mode_str);
-
-  if (!g_file_replace_contents (self->config_file,
-                                config_data->str,
-                                config_data->len,
-                                NULL, FALSE, 0, NULL,
-                                cancellable, error))
-    goto out;
-
-  if (!g_file_make_directory (self->objects_dir, cancellable, error))
-    goto out;
-
-  if (!g_file_make_directory (self->tmp_dir, cancellable, error))
-    goto out;
+  if (!glnx_opendirat (AT_FDCWD, repopath, TRUE, &dfd, error))
+    return FALSE;
 
-  {
-    g_autoptr(GFile) extensions_dir =
-      g_file_resolve_relative_path (self->repodir, "extensions");
-    if (!g_file_make_directory (extensions_dir, cancellable, error))
-      goto out;
-  }
+  if (fstatat (dfd, "config", &stbuf, 0) < 0)
+    {
+      if (errno == ENOENT)
+        {
+          const char *mode_str;
+          g_autoptr(GString) config_data = g_string_new (DEFAULT_CONFIG_CONTENTS);
 
-  g_clear_object (&child);
-  child = g_file_get_child (self->repodir, "refs");
-  if (!g_file_make_directory (child, cancellable, error))
-    goto out;
+          if (!ostree_repo_mode_to_string (mode, &mode_str, error))
+            return FALSE;
 
-  g_clear_object (&grandchild);
-  grandchild = g_file_get_child (child, "heads");
-  if (!g_file_make_directory (grandchild, cancellable, error))
-    goto out;
+          g_string_append_printf (config_data, "mode=%s\n", mode_str);
 
-  g_clear_object (&grandchild);
-  grandchild = g_file_get_child (child, "remotes");
-  if (!g_file_make_directory (grandchild, cancellable, error))
-    goto out;
+          if (!glnx_file_replace_contents_at (dfd, "config",
+                                              (guint8*)config_data->str, config_data->len,
+                                              0, cancellable, error))
+            return FALSE;
+        }
+      else
+        {
+          glnx_set_error_from_errno (error);
+          return FALSE;
+        }
+    }
 
-  g_clear_object (&child);
-  child = g_file_get_child (self->repodir, "state");
-  if (!g_file_make_directory (child, cancellable, error))
-    goto out;
+  for (guint i = 0; i < G_N_ELEMENTS (state_dirs); i++)
+    {
+      const char *elt = state_dirs[i];
+      if (mkdirat (dfd, elt, 0755) == -1)
+        {
+          if (G_UNLIKELY (errno != EEXIST))
+            {
+              glnx_set_error_from_errno (error);
+              return FALSE;
+            }
+        }
+    }
 
   if (!ostree_repo_open (self, cancellable, error))
-    goto out;
-
-  ret = TRUE;
+    return FALSE;
 
- out:
-  if (config_data)
-    g_string_free (config_data, TRUE);
-  return ret;
+  return TRUE;
 }
 
 static gboolean

diff --git a/tests/basic-test.sh b/tests/basic-test.sh
index 003df893367bd1bfb0a1c9c53955c45d855d71ef..d4c9aaf241fb1809bb38ab31a2d4f88705372c24 100755 (executable)
--- a/tests/basic-test.sh
+++ b/tests/basic-test.sh
@@ -19,7 +19,7 @@
 
 set -euo pipefail
 
-echo "1..57"
+echo "1..58"
 
 $OSTREE checkout test2 checkout-test2
 echo "ok checkout"
@@ -41,6 +41,12 @@ echo "ok shortened checksum"
 (cd repo && ${CMD_PREFIX} ostree rev-parse test2)
 echo "ok repo-in-cwd"
 
+rm test-repo -rf
+$OSTREE --repo=test-repo init --mode=bare-user
+$OSTREE --repo=test-repo init --mode=bare-user
+rm test-repo -rf
+echo "ok repo-init on existing repo"
+
 cd checkout-test2
 assert_has_file firstfile
 assert_has_file baz/cow